Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added an automated Selenium UI test for a small Zimit2 archive #1286

Open
wants to merge 31 commits into
base: main
Choose a base branch
from

Conversation

THEBOSS0369
Copy link
Collaborator

@THEBOSS0369 THEBOSS0369 commented Nov 11, 2024

This PR Fixes #1205

Hello Everyone!

Description

In this PR i have added an automated Selenium UI test for a small Zimit2 archive (used tonedear.com_en_2024-09.zim)

The tests i have added are as follows :

  1. Load Kiwix JS and verify title
  2. Switch to Jquery or SW mode
  3. Load Tonedear archive and verify content
  4. Navigate from main page to Android & iOS section
  5. Verify Android and iOS store images in Both Modes

Test

I have done all the necessary test

  1. Checked in both "Restricted" and "ServiceWorker" Everything is working fine check the ss below.
  2. Unit tests npm test no issue
  3. End-to-end (e2e) tests-e2e-firefox-> All tests Passed Below is the ss
  4. extension versions with production code tested
  5. Browser Test -> Microsoft Edge, Chrome, Firefox and Brave

Screenshot for tests

image

@THEBOSS0369
Copy link
Collaborator Author

THEBOSS0369 commented Nov 11, 2024

@Jaifroid I have not added test for android & ios page because i didn't know what test should i do, your suggestion will be appreciated 😅😃 Thanks!
!
image

@Jaifroid
Copy link
Member

Jaifroid commented Nov 13, 2024

Thank you @THEBOSS0369. That's very useful. I would suggest adding a test that switches to the page that shows the Android and iOS store images, and verifying that both images have loaded correctly:

  • In Restricted mode (aka JQuery), test that both images' src attribute has changed to a data URI (see screenshot), and assert that both data URIs match the beginning of the expected content (say 30 characters, so you get the beginning of the actual data, not just "data:img/png,base64,". If you want to be thorough, you could hash the string and test the hashes, but that may be overkill.
  • In ServiceWorker mode, you can do a basic image verification: check if the image has completed loading using JavaScript properties, and verify that the image has natural dimensions (width/height > 0). Use the computed naturalWidth and naturalHeight properties of the images to verify (see https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/naturalWidth). This will tell us that the image loaded correctly (after appropriate wait).

image

@Jaifroid Jaifroid added the tests label Nov 13, 2024
@Jaifroid Jaifroid added this to the v4.2 milestone Nov 13, 2024
@THEBOSS0369
Copy link
Collaborator Author

THEBOSS0369 commented Nov 15, 2024

Hey @Jaifroid ! I have added the android and ios test as you asked me but there is a problem occurring when i am running the tests the restricted mode test passes well but in sw mode this error comes
android test failing

i have tried by making the waititme 120000 , by removin cache but still this won't work. I don't understand what i am doing wrong i checked in the sw mode the image's alt is present and i have added the same in the code. Your help will be appreciated . Thanks!

@Jaifroid
Copy link
Member

Thanks @THEBOSS0369. Sorry to hear about the error. It's tricky to get the assertions right with the timing. I'll take a look at the weekend, but in the meantime it's possibly the case that there's some error in selecting the images and checking their natural width. Adding some console logging to see what it's actually finding and what you are expecting it to find might help!

@THEBOSS0369
Copy link
Collaborator Author

THEBOSS0369 commented Nov 16, 2024

Yes sir. I'm doing that by further more progress i have found that there are in total 21 images showing 19 are icon_external_link one is gif and one is kiwix logo this script is still ain't able to find the store images. i will try to find more in the meanitime maybe i find something useful ,right now this is what's showing.
image

Note: I didn't pushed the code as it has become very messy so if you want to see the code just tell me i will push it. 😃🙂

@Jaifroid
Copy link
Member

OK, I ran this myself (in the Edge runner, adding the appropriate commands), and while watching the browser locally it's clear that the test never actually loads the page "Android & iOS App". It remains stuck on "Ear Training Practice" (see screenshot). You're not going to be able to verify the images on the "Android & iOS App" page unless you actually switch to it first!

image

Copy link
Member

@Jaifroid Jaifroid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see below for the steps to remedy the issue you're encountering.

@@ -27,6 +28,8 @@ async function loadFirefoxDriver () {
// so we need to use the second one first
const driver_for_gutenberg = await loadFirefoxDriver();
const driver_for_ray_charles = await loadFirefoxDriver();
const driver_for_tonedear = await loadFirefoxDriver();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the comment says above this block, the drivers need to be listed in reverse test order here, so that when the user runs the tests locally we see the Ray Charles first (which will be on top of the other windows). So please put the tonedear test above gutenberg in this list, and update the comment to reflect that there are three tests

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done Sir

tests/e2e/spec/tonedear.e2e.spec.js Show resolved Hide resolved
@THEBOSS0369
Copy link
Collaborator Author

THEBOSS0369 commented Nov 17, 2024

@Jaifroid Another problem occurring , when i try to navigate to Android Page it won't, i think this is happening because i didn't know the exact path for it at first i used this android-ios-ear-training-app it didn't worked i tried by adding /android-ios-ear-training-app & ../android-ios-ear-training-app and many more tries but still didn't worked
image

then i looked into the ray_charles code as it also have a navigation function i tried this which is
const appLink = await driver.wait(until.elementLocated(By.xpath("//ul[contains(@class, 'navbar-nav navbar-right')]/li[1]/a")), 20000);
similar to that present in ray chares
articleLink = await driver.findElement(By.xpath('/html/body/div/div/ul/li[77]/a[2]'));
but still not working.

Then i think i can do that by link but The link for the server remains same
http://localhost:5173/www/index.html?appCache=false
even when i open any other page but when i run the tests it have a new url
http://localhost:8080/dist/www/index.html?title=C/tonedear.com/android-ios-ear-training-app
for different pages i tried by clearing cache and using incognito mode still url remains same. I'm not sure if this is default or not.

Can you help me with this path problem if we get it fixed then every tests will pass correctly.

@Jaifroid
Copy link
Member

@THEBOSS0369 Oh no, this app doesn't support navigating to a page by URL fragment. There are two ways to get to a particular page: either click on the link that you highlight in the screenshot (easiest), or search for it using the search bar. Also, remember that the links you're clicking on are in the iframe, not at the top level, so you have to switch to the iframe before finding the link and clicking it. Take a look at the code in the Ray Charles test to see how to switch to the iframe.

This explains why you said you were finding 21 images in the landing page when in fact there are none! You were finding these images in the app itself, as you hadn't switched to the iframe containing the content.

@THEBOSS0369
Copy link
Collaborator Author

@Jaifroid Sirrr ! it worked now its navigating to andorid page although the images are still not found now i will try to fix that
image
image

@THEBOSS0369
Copy link
Collaborator Author

@Jaifroid Sir all The tests have passed . Does any thing else needed to be added in the test? If yes then do let me know.

The comments in this image for tonedear tests will not be visible in the main tests i have commented it out.
image

@Jaifroid
Copy link
Member

@THEBOSS0369 That's very good news! I don't think we need more tests. I'll review the new code as soon as I can, but in the meantime, you might want to clean it up, remove any excessive timeouts you previously added, etc. Once the spec is ready, you should add it to all the other runners in the folders under tests/e2e/runners. Then I'll need to run it on BrowserStack to be sure the tests pass on all browsers.

Oh, I see GitHub tests have failed. I'll re-run those to be sure it wasn't a timeout fluke.

@THEBOSS0369
Copy link
Collaborator Author

THEBOSS0369 commented Nov 17, 2024

@Jaifroid I think the tests are failing because when it navigate to android page it asks me to authorize the author and i used to press that maybe that's causing error.
image

@THEBOSS0369
Copy link
Collaborator Author

THEBOSS0369 commented Nov 17, 2024

@Jaifroid Yes this is happening due to the reason i told above i didn't clicked the trust source this time and tests failed. I don't know how to fix this one
image

@Jaifroid
Copy link
Member

Jaifroid commented Nov 17, 2024

Yes this is happening due to the reason i told above i didn't clicked the trust source this time and tests failed. I don't know how to fix this one

This is because you erased https://github.com/kiwix/kiwix-js/blob/main/tests/e2e/spec/legacy-ray_charles.e2e.spec.js#L125 (the setting "noPrompts=true") from the URL when loading the app. This was in your previous spec, but then you did a hard reset of your branch, and you erased it, so it's no longer in the commit history.

That command prevents the security prompt from showing. All you need to do is copy the appropriate code block from the ray charles version of the test.

Also, in future, try to avoid rewriting history in your PRs. It makes things difficult to review as well, because when you do a force push, I have to reset my copy of your PR instead of simply pulling a new commit! (Don't worry, everyone makes these kinds of mistakes with PRs when learning.)

@THEBOSS0369
Copy link
Collaborator Author

THEBOSS0369 commented Nov 18, 2024

@Jaifroid Done Sir! Now all tests passes well without any issue.
image

I'm waiting for these tests to pass then i will add the tests in all the files and remove any unnecessary time waits.

Also, in future, try to avoid rewriting history in your PRs. It makes things difficult to review as well, because when you do a force push, I have to reset my copy of your PR instead of simply pulling a new commit! (Don't worry, everyone makes these kinds of mistakes with PRs when learning

My apologies Sir i didn't knew force push can do this much damage. I will be careful from next time 😅

@THEBOSS0369
Copy link
Collaborator Author

THEBOSS0369 commented Nov 18, 2024

I have increased the timewait maybe this can fix it
image

Edit (11:10): All the tests have been passed so i'm reducing time wait

Edit Again (12:30): Sir i have added the tests in all the files .

@Jaifroid
Copy link
Member

Jaifroid commented Dec 4, 2024

Thank you! I'm just running the BrowserStack tests.

@THEBOSS0369
Copy link
Collaborator Author

@Jaifroid sir the tests are failing so is it the same dialog box issue this time because its getting timed out ?

@Jaifroid
Copy link
Member

Jaifroid commented Dec 4, 2024

Take a look here: https://automate.browserstack.com/builds/73815a6c7951be3a00e49ef6321ac1855566c1ba/sessions/3e44e115671d72df8d8197bc551749f5a59c1d1d?auth_token=6f699a3ef8e859020aea94c8de6996e4b1719aca8944a9e5ed0abe9b0bf40972

There seems to be an error Unable to locate element: .modal[style*="display: block"]. However, I'm going to rerun the test in case it was a timeout issue.

@Jaifroid
Copy link
Member

Jaifroid commented Dec 4, 2024

Oh, I have an idea: you need to handle the case that the dialogue box ISN'T shown. Don't assume it will be there...

In principle, it shouldn't be shown at all, due to a querystring we add 'noPrompt=true`. I wonder if that querystring was simply missing in the previous error you fixed? Because it's not missing in this test, the prompt doesn't show.

@Jaifroid
Copy link
Member

Jaifroid commented Dec 4, 2024

What I'm saying is, check over your previous fix to see if the issue was that you were calling the app without noPrompt=true, and that was why the prompt dialogue box appeared. In that case, you could fix this by removing the prompt handling code and ensuring the app is always loaded with that querystring.

@THEBOSS0369
Copy link
Collaborator Author

@Jaifroid Sir I don't understand what's the problem here. let's divide all the possibilities

  1. First For querystring

In principle, it shouldn't be shown at all, due to a querystring we add 'noPrompt=true`. I wonder if that querystring was simply missing in the previous error you fixed? Because it's not missing in this test, the prompt doesn't show.

No sir , i didn't changed this since the first commit this is the code which is present since the start

            it('Load Kiwix JS and verify title', async function () {
                await driver.get('http://localhost:' + port + '/dist/www/index.html?noPrompts=true');
                await driver.sleep(1300);
  1. The fix that i used for the dialog box to not appear is this, it was present in the day before yesterday's commit
            it('Navigate from main page to Android & iOS section', async function () {
                // Check for Dialog Box and click any Approve Button in subsequent dialog box
                try {
                    const activeAlertModal = await driver.findElement(By.css('.modal[style*="display: block"]'));
                    if (activeAlertModal) {
                        // console.log('Found active alert modal');
                        const approveButton = await driver.findElement(By.id('approveConfirm'));
                        await approveButton.click();
                    }
                } catch (e) {
                    // Do nothing
                }

                // Switch to the iframe if the content is inside 'articleContent'
  1. Sir i don't why but i had already added this code in my very first commit still the dialog box was appearing in the browser stack test which you asked me to fix
    image

I have got confused please clarify this sir 😓

@Jaifroid
Copy link
Member

Jaifroid commented Dec 4, 2024

Yes, I know you already added the code, but it chokes on looking for a dialogue box that didn't appear this time. I'm not sure why the dialogue box appeared the previous time -- my thought was that maybe the querystring telling it not to show was ommitted, but I didn't check the code.

Did you see it this time? When I looked at the video, I didn't see the security diaologue box, and so the script got stuck because it couldn't find it.

In any case, if it's sometimes appearing and sometimes not, your code needs to handle the situation in which it's not there. I.e., it should dismiss the dialogue if found, but just continue if not found within a certain amount of time.

@THEBOSS0369
Copy link
Collaborator Author

Did you see it this time? When I looked at the video, I didn't see the security diaologue box, and so the script got stuck because it couldn't find it.

No sir i also didn't see any dialogue box appearing in the video.
this time i have added the code to check whether dialogue box appears or not and will work according to that let's see if this time tests passes well or not.

@Jaifroid
Copy link
Member

Jaifroid commented Dec 5, 2024

Still failing, I'm afraid. See https://github.com/kiwix/kiwix-js/actions/runs/12176007591/job/33960839227?pr=1286 . It correctly recognizes there is no dialogue box, and then fails to navigate to the iOS/Android page, apparently. In fact, it looks like you don't load the landing page at all? The app remains stuck on Configuration. Do you even load the ZIM, it doen's tlook like it? Perhaps because this is a ServiceWorker mode only test, you accidentally skip the loading of the ZIM (I'm just guessing).

See BrowserStack report here for more info: https://automate.browserstack.com/builds/73815a6c7951be3a00e49ef6321ac1855566c1ba/sessions/088eed0cf1ab6ec43ecd5b2b4075a2607e7d0fb1?auth_token=0611b644be6ed3693eaa3cfd5f1e5ba1d16a364388143ff06da8a70c9a45bceb .

@THEBOSS0369
Copy link
Collaborator Author

@Jaifroid Sir i'm not sure why its happening i am going to do some changes and test it using only sw mode but right now in my local server the landing pages is loading correctly please see the video below for reference
https://github.com/user-attachments/assets/dc2cdbaa-3a42-4955-96e6-912443b67e84

@Jaifroid
Copy link
Member

Jaifroid commented Dec 5, 2024

@THEBOSS0369 I think the difference is that in your local test you are running it first in Restricted mode (jQuery), and then in SW mode. In the affected test, only SW mode is called. Try replicating that locally, i.e. call the test with await tonedear.runTests(driver_tonedear_fx, ['serviceworker']);. See if it fails locally and fix if necessary.

@THEBOSS0369
Copy link
Collaborator Author

@Jaifroid sir solo sw mode tests are pasing as well please see the video

Test.fail.sw.mode.mp4

@THEBOSS0369
Copy link
Collaborator Author

THEBOSS0369 commented Dec 5, 2024

@Jaifroid sir could be this be a browser specific issue because in latest version tests are not failing, so should we try it first in different browsers if this tests fail on those one as well then this issue is definitely in the code and if it passes in others then we can know why the problem's occuring ?

@Jaifroid
Copy link
Member

Jaifroid commented Dec 5, 2024

OK. Another thing I noticed in the BrowserStack video was that the app reloaded itself just before tests started. That suggests it was having trouble loading and registering the Service Worker (it will keep reloading itself until the SW is registered correctly). It may be you need to increase the wait between loading the app's URL and starting the tests, to give the SW more time to register.

You can of course experiment with skipping this test for now, so that the next ones run, then we'll know if it's a specific browser issue or a more general problem.

@Jaifroid
Copy link
Member

Jaifroid commented Dec 5, 2024

Running tests.

@THEBOSS0369
Copy link
Collaborator Author

@Jaifroid sir I think the tests are failing due to the latest changes i made let me revert it back and see if it works and if not then i will have to do some experiments. Also sir is there any other way that i can run these tests myself and i don't have to disturb u again & again .

@THEBOSS0369
Copy link
Collaborator Author

sir i have reverted all changes and tets are passing in my local let's see if it passes this time or not

@Jaifroid
Copy link
Member

Jaifroid commented Dec 6, 2024

@THEBOSS0369 Unfortunately this is now failing on Chrome 60. So it looks like it wasn't browser-specific. Here are the two failed runs:

I can give you write access to the Repo so that the BrowserStack tests run without my needing to intervene. However, my feeling is that the code to dismiss the security prompt may actually be the problem. As mentioned, if called correctly with the noPrompts querystring this prompt should never show, so we shouldn't need to have code dealing with it. I would try removing that, and then if we find that there is a security prompt, we can fix that with the npPrompts...

@Jaifroid
Copy link
Member

Jaifroid commented Dec 6, 2024

@THEBOSS0369 An invite has been sent to you to accept write access to the Repo. Use this with extreme responsibility. It should mean the BrowserStack tests run when you push (after you've accepted the invite). I can give you access to the BrowserStack system so you can look at the videos for yourself. For that, please send me a private message on the Kiwix Slack.

@Jaifroid
Copy link
Member

Jaifroid commented Dec 6, 2024

(You may need to push directly to the Repo for the BS tests to run, rather than pushing from a fork, I'm not 100% certain. Try pushing as normal, and if it doesn't work I can explain how to push directly to a branch, so the tests run.)

@THEBOSS0369
Copy link
Collaborator Author

@THEBOSS0369 An invite has been sent to you to accept write access to the Repo. Use this with extreme responsibility. It should mean the BrowserStack tests run when you push (after you've accepted the invite). I can give you access to the BrowserStack system so you can look at the videos for yourself. For that, please send me a private message on the Kiwix Slack.

Got it sir i will do the pushing with extreme care without touching anything else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an automated Selenium UI test for a small Zimit2 archive
2 participants